-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
stream: add ChunksTimeout::into_remainder
#7715
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Use caseuse std::time::Duration;
use tokio::sync::mpsc;
use tokio_stream::{StreamExt, wrappers::ReceiverStream};
#[tokio::main]
async fn main() {
let (tx, rx) = mpsc::channel::<u64>(20);
let mut receiver = ReceiverStream::new(rx);
let chunked = (&mut receiver).chunks_timeout(3, Duration::from_secs(3));
tokio::pin!(chunked);
tx.send(10).await.unwrap();
tx.send(20).await.unwrap();
tokio::select! {
Some(batch) = chunked.next() => {
println!("Got: {:?}", batch);
}
_ = tokio::time::sleep(Duration::from_secs(1)) => {
// another condition
}
}
let remainder = chunked.into_remainder();
// use `receiver` here
} |
aaa757e to
0fb2203
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a similar interface in the standard library: std::slice::ChunksExactMut:into_remainder. This might be a better interface for your requirements. What do you think?
|
Thank you @ADD-SP for your review. The original plan was to completely consume the stream indeed and just return the reminder. This however won't work with the Pinned ChunksTimeout stream which is required when polling next() in a select statement. This means an I will still rename the method to |
f2c8ed1 to
4187d7b
Compare
|
Thank you so much @ADD-SP for your review. I applied all your comments. Let me know if you have more comments. |
|
I think you could add the code example from #7715 (comment) as a snippet in the rustdoc for the new method. Some new tests would be nice too! |
@martin-g That example seems too long, and it’s not easy for downstream readers to grasp it in a few seconds. I’m okay with adding an example, but if we do, it should be simpler than the existing one. |
ADD-SP
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, like what @martin-g said, please add tests for this new feature.
ChunksTimeout::into_reminder
ChunksTimeout::into_reminderChunksTimeout::into_remainder
ChunksTimeout::into_remainderd09dc64 to
fa42216
Compare
|
Dear @ADD-SP, thank you for your time! I’ve added a test case demonstrating how I would use it mainly to recover any buffered remainder, then restore the inner stream and continue consuming it without losing any items. The example is a simplified version of the test, though I’m not sure if it’s simple enough for readers. Let me know what you think or if you have any suggestions to improve it |
19d513b to
9a6783e
Compare
ADD-SP
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
@ADD-SP sorry about changing the title, that was indeed an accident. I use sapling which pushes title and description when submitting the PR. I will use the title you suggested. |
d689129 to
b464cb5
Compare
Summary: When the underlying stream is an exclusive reference (&mut stream), and we need to drop the `ChunksTimout` stream without losing the buffered items.
|
Dear @ADD-SP thank you so much for your tips and pointers. I like the tests much more now with the paused-time. Great feature that I have overlooked 😄 Let me know if you still have any more comments. |
ChunksTimeout::into_remainder
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have considered two different interfaces for this usecase.
into_remainder
pub fn into_remainder(self: Pin<&mut Self>) -> Vec<T>;Pros
- This aligns to
std::slice::ChunksExactMut:into_remainder.
Cons
- The receiver is
self: Pin<&mut Self>instead ofself, which is a bit different from thestd::slice::ChunksExactMut:into_remainder. - The downstream can keep using it after calling into_remainder(). The semantics look a bit strange.
pinned.as_mut().into_remainder(); pinned.as_mut().into_remainder();
take_remainder
pub fn take_remainder(self: &mut Pin<&mut Self>) -> Vec<T>;Pros
- The semantic looks more straightforward for downstream to keep using it after calling the
take_remainder.let elems = pinned.take_remainder(); let _ = pinned.poll_next(&mut cx);
Cons
- The
&mut Pin<&mut Self>is not a common pattern in Rust.
Why choose into_remainder?
Because of the use case. The use case of this interface is to retrieve buffered items after .chunked_timeout().await is canceled, and the downstream is no longer going to keep polling it. For example:
tokio::select! {
Some(batch) = chunked.next() => todo!(),
_ = tokio::signal::ctrl_c() => (),
}
let elems = chunked.into_remainder();For this use case, into_remainder() is better than take_remainder() semantically, because the downstream is not going to poll the chunked again, so consuming it makes more sense.
|
Thanks! |

stream: add ChunksTimeout::into_remainder
Summary:
When the underlying stream is an exclusive reference (&mut stream), and we need to drop the
ChunksTimoutstream without losing the buffered items.